[release] Allow release test to specify num_retries #57578
Conversation
Signed-off-by: kevin <[email protected]>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a useful feature to configure the number of retries for release tests. The implementation is straightforward, but I've found one important issue in the logic that handles the num_retries value. Please see my comment below.
| cmd += ["--smoke-test"] | ||
|
|
||
| num_retries = test.get("run", {}).get("num_retries") | ||
| if num_retries: |
There was a problem hiding this comment.
The condition if num_retries: will evaluate to False when num_retries is 0. This prevents disabling retries for a test by setting num_retries: 0, which is a key use case mentioned in the PR description ("to not retry"). When num_retries is 0, the default retry limit (likely 1) will be used instead of disabling retries.
To correctly handle the case where num_retries is 0, you should check if the value is not None.
| if num_retries: | |
| if num_retries is not None: |
| if smoke_test: | ||
| cmd += ["--smoke-test"] | ||
|
|
||
| num_retries = test.get("run", {}).get("num_retries") |
ray-gardener
bot
added
core
|
This pull request has been automatically marked as stale because it has not had You can always ask for help on our discussion forum or Ray's public slack channel. If you'd like to keep this open, just leave any comment, and the stale label will be removed. |
github-actions
bot
added
the
stale
Signed-off-by: kevin <[email protected]>
github-actions
bot
added
unstale
commits.txt
Outdated
| @@ -0,0 +1,315 @@ | |||
| [core][cherry-pick] RDT NIXL Limitations + Throw Exception (#58159) | |||
There was a problem hiding this comment.
remove this file? not related?
There was a problem hiding this comment.
oops it was the list of commits for release.. removed now
Signed-off-by: kevin <[email protected]>
… khluu/release_retry
So it can be configured for expensive tests to not retry --------- Signed-off-by: kevin <[email protected]> Co-authored-by: Lonnie Liu <[email protected]>
So it can be configured for expensive tests to not retry --------- Signed-off-by: kevin <[email protected]> Co-authored-by: Lonnie Liu <[email protected]>
So it can be configured for expensive tests to not retry --------- Signed-off-by: kevin <[email protected]> Co-authored-by: Lonnie Liu <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
So it can be configured for expensive tests to not retry